Skip to content
This repository has been archived by the owner on Aug 1, 2023. It is now read-only.

Validate usernames #19

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
Open

Validate usernames #19

wants to merge 12 commits into from

Conversation

hstove
Copy link
Collaborator

@hstove hstove commented Jul 24, 2019

This PR implements the proposal to validate usernames associated with particular models. If a username is present with a model, then the validator will:

  1. Lookup the profile.json associated with that username
  2. Make sure that the Gaia URL used to save data is in this profile

This is a somewhat roundabout way of proving ownership of a given name. If you can write to a specific gaia URL, and that gaia URL is in your profile.json, it follows that these users are the same.

cc @moxiegirl, @pradel , @friedger who have expressed interest here.

See stacks-archive/radiks#44 for the front-end code update, which is minimal.

I have published a beta version for testing - 1.1.0-beta.1

TODO:

  • Add some caching to the fetch of profile.json. Right now, it is done in a very inefficient way, and fetches the profile every time.
  • Only check the 'address' portion of the Gaia URL. This would better handle the case where a user migrates to a different Gaia hub.
  • Validate that gaiaURL is present if username is present
  • Include validateUsername on all models, unless it has a userGroupId (to preserve group privacy)
  • Special logic for usernames that don't exist as Blockstack IDs yet

* the Gaia URL to any Gaia URL in that user's profile.json
*/
async validateUsername(): Promise<boolean> {
if (!(this.attrs.username && this.gaiaURL)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is it possible that the gaiaURL is not set? If it is not set should then username be null so that it is not possible to create a model with a username that is not verified?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The gaiaURL is set always on the client side if validateUsername is true. This should be updated to also validate for the presence of gaiaURL. Good catch, I'll add to the "todos"

@hstove
Copy link
Collaborator Author

hstove commented Jul 30, 2019

Hi all, I've added 2 tasks to add to this PR:

  • Validate that gaiaURL is present if username is present. Suggestion from @friedger
  • Include validateUsername on all models, unless it has a userGroupId (to preserve group privacy). This is in line with the spec.

@hstove hstove force-pushed the feature/validate-usernames branch from 1bb4414 to 402b11c Compare July 30, 2019 14:54
@hstove
Copy link
Collaborator Author

hstove commented Oct 15, 2019

One issue that @leopradel brought up was that this doesn't work well for brand-new Blockstack users. If they just registered a name, and it hasn't propagated yet, then how can we validate the username?

One option is to accept User model registrations if they're using a Blockstack ID that does not exist. If it does exist, we'll do the same validation that we do here.

@friedger
Copy link
Collaborator

friedger commented Feb 23, 2020

Include validateUsername on all models

There is more talks about blockstack ids without username. Whether a username is required should be a validation done on the client of the radiks server, shouldn't ?

@friedger
Copy link
Collaborator

friedger commented Feb 23, 2020

If they just registered a name, and it hasn't propagated yet, then how can we validate the username?

Same holds for invalid usernames that were never registered.
Ideally, models that contain the username (and needs to be verified) are signed with the private key of the username. As long as there is no way to verify that the current user owns the key of the username models shouldn't verify the models.

I suggest that the server removes username from the unverifiable model and stores the value in usernameUnverified or so.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants